Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[stdlib] Add string benchmarking infrastructure and basic benchmarks #3523

Closed
wants to merge 21 commits into from
Closed

[stdlib] Add string benchmarking infrastructure and basic benchmarks #3523

wants to merge 21 commits into from

Conversation

martinvuyk
Copy link
Contributor

Add string benchmarking infrastructure and basic benchmarks.

Added data for benchmarking collections

UN charter:
Taken from the official UN website with the language abbreviations following ISO 639-1 except simplified mandarin chinese (zh-CN)

@martinvuyk
add string benchmarking infrastructure and basic benchmarks
5450fd5 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk martinvuyk requested a review from a team as a code owner September 22, 2024 19:39
martinvuyk and others added 9 commits September 22, 2024 15:41
@martinvuyk
fix detail
29d946f 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
e9b568a 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
try fix markdownlint
5eb3dc3 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
9740905 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
86efad7 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
152cd26 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
a6c9279 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
fix detail
3b37440 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
Merge branch 'nightly' into add-string-benchmark-infra
340e6ba
@martinvuyk martinvuyk mentioned this pull request Sep 24, 2024
Open
1 task
@JoeLoser JoeLoser self-assigned this Oct 13, 2024
martinvuyk and others added 2 commits October 14, 2024 10:15
@martinvuyk
Merge branch 'nightly' into add-string-benchmark-infra
b93ba51
@martinvuyk
fix detail
5783dd7 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
JoeLoser
JoeLoser approved these changes Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, nice work!

Did you intend to check in the HTML files? I think it suffices to just have the txt files that get read in the benchmarks.

@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Oct 14, 2024
@martinvuyk
Copy link
Contributor Author

martinvuyk commented Oct 14, 2024 •
edited
Loading

Did you intend to check in the HTML files?

Yeah my thoughts were that it is a more realistic payload for things like utf8 validation (that benchmark uses the htmls) and any other stuff we build later on with parsing (xml, json) and anything to do with sockets (if we wanna measure realistic throughput, latency, and resource utilization not a toy test with some 5 line json). I have no problem leaving them out and include when necessary, I just thought it's stuff that is often forgotten later on when trying to benchmark and that it would be better to set a strong baseline as default WDYT?

@martinvuyk
Copy link
Contributor Author

@JoeLoser I think it's fixed (currently running another heavy load on my machine so can't run the benchmarks, but it compiles). It was just replacing usages of String in the parameters to StringLiteral

@martinvuyk
fix all problems
352aca8 
Signed-off-by: martinvuyk <martin.vuyklop@gmail.com>
@martinvuyk
Copy link
Contributor Author

@JoeLoser now they actually run, the @parameter for was trying to unroll all lengths and took forever to compile, so I just reduced the amount. Also the part to read files was wrong

@JoeLoser
Copy link
Collaborator

!sync

@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added the merged-internally Indicates that this pull request has been merged internally label Oct 22, 2024
modularbot added a commit that referenced this pull request Oct 23, 2024
@modularbot @martinvuyk
[External] [stdlib] Add string benchmarking infrastructure and basic …
b75debe 
…benchmarks (#49095)

[External] [stdlib] Add string benchmarking infrastructure and basic
benchmarks

Add string benchmarking infrastructure and basic benchmarks.

Added data for benchmarking collections.

UN charter:
Taken from the official [UN
website](https://www.un.org/en/about-us/un-charter/full-text) with the
language abbreviations following ISO 639-1 except simplified mandarin
chinese (zh-CN)

ORIGINAL_AUTHOR=martinvuyk
<110240700+martinvuyk@users.noreply.github.com>
PUBLIC_PR_LINK=#3523

Co-authored-by: martinvuyk <110240700+martinvuyk@users.noreply.github.com>
Closes #3523
MODULAR_ORIG_COMMIT_REV_ID: d3b154bc993eccf4b80bb3773895bc28e1f41ee6
@modularbot
Copy link
Collaborator

Landed in b75debe! Thank you for your contribution 🎉

@modularbot modularbot added the merged-externally Merged externally in public mojo repo label Oct 23, 2024
@modularbot modularbot closed this Oct 23, 2024
@martinvuyk martinvuyk deleted the add-string-benchmark-infra branch October 23, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLoser JoeLoser JoeLoser approved these changes

Assignees

@JoeLoser JoeLoser

Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinvuyk @JoeLoser @modularbot